-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Illumos 6815179, 6844191, 5518, 1778 #4252
Conversation
If I understand correctly, zpool import will be magnitude faster with this. Any numbers ? |
5d21e82
to
cf1f598
Compare
@odoucet yes that's the idea. How much of a speed up depends entirely on how many entries exist under /dev/ to be scanned. As a test I created 500 symlinks under /dev/ to scan. This roughly corresponds to a pool with 32 disks since each disk will have 2 partitions (plus the base device) and 5 symlinks to each entry is common (32 * 3 * 5 = 480). In this configuration I saw the import time drop from ~5.71 seconds to ~0.36. That's roughly a 15x improvement so this patch definitely does help. As always your mileage may vary. |
OK great. Considering your startup time, I assume you have very small amount of zfs volumes or snapshots. |
@odoucet this change this just optimizes the drive scanning so it's not going to help in your case. But there are other which were being developed speed up the issue your seeing. |
Are you referring to Illumos 5269 ? |
No, actually #3830 which is overdue for a code review and some testing. |
cf1f598
to
33492e4
Compare
6815179 zpool import with a large number of LUNs is too slow 6844191 zpool import, scanning of disks should be multi-threaded References: illumos/illumos-gate@4f67d75 Porting notes: - This change was originally never ported to Linux due to it dependence on the thread pool interface. This patch solves that issue by switching the code to use the existing taskq implementation which provides the same basic functionality. However, in order for this to work properly thread_init() and thread_fini() must be called around to taskq consumer to perform the needed thread initialization. - The check_one_slice, nozpool_all_slices, and check_slices functions have been disabled for Linux. They are difficult, but possible, to implement for Linux due to how partitions are get names. Since this is only an optimization this code can be added at a latter date. Signed-off-by: Brian Behlendorf <[email protected]>
5518 Memory leaks in libzfs import implementation Reviewed by: Dan Fields <[email protected]> Reviewed by: Matthew Ahrens <[email protected]> Reviewed by: Serghei Samsi <[email protected]> Approved by: Dan McDonald <[email protected]> References: https://www.illumos.org/issues/5518 illumos/illumos-gate@078266a Porting notes: - One hunk of this change was already applied independently in commit 4def05f. Signed-off-by: Brian Behlendorf <[email protected]>
1778 Assertion failed: rn->rn_nozpool == B_FALSE Reviewed by: Matthew Ahrens <[email protected]> Reviewed by: George Wilson <[email protected]> Reviewed by: Richard Elling <[email protected]> Approved by: Gordon Ross <[email protected]> References: https://www.illumos.org/issues/1778 illumos/illumos-gate@bd0f709 Signed-off-by: Brian Behlendorf <[email protected]>
33492e4
to
8e14d80
Compare
Port the parallel zpool scanning code and subsequent changes from Illumos to Linux. This was skipped long ago since it was never critical and it depended on the illumos specific thread_pool interface. We definitely don't want to support yet another compatibility interface, luckily the taskq interfaces are almost identical. Therefore, the patch was updated to use taskqs instead. Additional porting comments can be found in the commit comments.
Illumos 1778 - Assertion failed: rn->rn_nozpool == B_FALSE
Illumos 5518 - Memory leaks in libzfs import implementation
Illumos 6815179, 6844191